Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check whether the mousedown event belonged to a valid drop target before preventing default #1778

Merged
merged 1 commit into from
May 8, 2014

Conversation

andrewnicols
Copy link
Contributor

This issue affects 3.16.0 (yay).
Sorry for the spam on the previous issue. I have yet to get hub to make sensible pull requests against non-master branches.

@yahoocla
Copy link

CLA is valid!

@andrewnicols
Copy link
Contributor Author

Note, I did try and write some tests for this, but I'm not sure how best to simulate an actual mousedown. Simulate simulates the events rather than the actual mousedown so it's not possible to check whether the field was properly focused.

@triptych
Copy link
Contributor

//cc @tilomitra for review?

andrewnicols added a commit to andrewnicols/moodle that referenced this pull request Apr 22, 2014
@andrewnicols
Copy link
Contributor Author

@tilomitra - any chance you can look at this. we have a release in around 2-3 weeks and I'd really like to backport this fix into our repo before then.

@andrewnicols
Copy link
Contributor Author

Also note, this is a major breakage. Any page where DD is enabled will not be gesture scrollable by a touch-based device.

@robframpton
Copy link

This also fixes a regression caused by 9f96abc where inputs cannot receive focus via mousedown if they are nested inside of a drag node (see here http://codepen.io/Robert-Frampton/pen/FhdmD).

@andrewnicols
Copy link
Contributor Author

@Robert-Frampton that's exactly the bug it's fixing.

@andrewnicols
Copy link
Contributor Author

@triptych, @tilomitra - any chance of a review on this - it's now been three weeks.

@marclundgren
Copy link
Contributor

@andrewnicols +1 for this fix

@triptych
Copy link
Contributor

triptych commented May 4, 2014

I asked @tilomitra to take a look. I'll follow up on Monday.

@tilomitra
Copy link
Contributor

Hey guys - sorry I was heads-down on some stuff last week. Taking a look at this now @andrewnicols. Will get back to you by EOD.

@ezequiel
Copy link
Contributor

ezequiel commented May 8, 2014

It's a shame this took so damn long to "review."

@andrewnicols,

Thanks. This looks great.

@ezequiel ezequiel merged commit f89052f into yui:dev-3.x May 8, 2014
@tilomitra
Copy link
Contributor

Thanks @ezequiel. Been swamped :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants